-
Notifications
You must be signed in to change notification settings - Fork 27.4k
orderBy should not reverse an array of objects #9566
Conversation
When no predicate is provided, orderBy will reverse an array of objects. This isn't ideal. It should err on the side of preserving the order. It happens because, for objects, in the compare() function, v1 < v2 is false, so the comparator function returns 1 (effectively moving v2 before v1).
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
Hmmm... I signed the CLA earlier today. The email address should match... |
Your commit is authored with lolson@... --- the CLA signature verifier looks at the author email address. You can change it with |
Thanks for being so patient with me. I just added that email to the CLA instead. Figured it would be easier. Let me know if that worked. |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
Looking more into this, it's actually not a good fix for the given issue, because of the different ways browsers implement sorting. What should happen is this: When no predicate is provided, the array's contents are checked, and if the array contains objects, no sorting should be done. |
you are correct. different vms compare objects differently so this PR is flawed. I don't understand why you use orderBy filter in this situation at all. |
Sure, let me explain my use case, and maybe you could suggest a different way to do it. I have a table, where each row corresponds to an object, like so:
Before a user clicks on a header cell, there is no predicate (it is |
4dd5a20
to
998c61c
Compare
We have a very similar use case for orderBy in tables like already descriped by @lukasolson I would appreciate that orderBy donsn't order a given array as long as predicate and reverse order are undefined. |
maybe the easiest solution is to set orderBy defaults to "+ " (plus and space) instead of "+". This will work just fine |
I am also working with large dataset and with m48u fix, it started to work fine in all browsers except for Chrome where it shows wrong first row ( replacement of some random row from following rows ) when the data is first loaded using ng-repeat.Not sure , why it wouldn't work in Chrome only.I tested on FF,Safari,IE and Chrome |
I added a PR (#9747), please take a look. |
Thanks for the PR, but for me it seems that you're missing the point. By setting predicate to an empty string if not provided the order will end up reversed (same as "+"), which is IMHO wrong. |
…t provided In ES262, there are two properties which are used to get a primitive value from an Object: - valueOf() -- a method which returns a primitive value represented by the Object - toString() -- a method which returns a string value representing the Object. When comparing objects using relational operators, the abstract operation ToPrimitive(O, TypeHint) is used, which will use these methods to retrieve a value. This CL emulates the behaviour of ToPrimitive(), and ensures that no ordering occurs if the retrieved value is identical. This behaviour was previously used for Date objects, however it can be safely made generic as it applies to all objects. Closes angular#9566 Closes angular#9747
When no predicate is provided, orderBy will reverse an array of objects.
This isn't ideal. It should err on the side of preserving the order.
It happens because, for objects, in the compare() function, v1 < v2 is
false, so the comparator function returns 1 (effectively moving v2
before v1).